Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Split logic of ExerciseResource into endpoints per exercise-type #9570

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ole-ve
Copy link
Contributor

@ole-ve ole-ve commented Oct 23, 2024

This is a draft, feel free to ignore :)

@ole-ve ole-ve self-assigned this Oct 23, 2024
@github-actions github-actions bot added server Pull requests that update Java code. (Added Automatically!) exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module fileupload Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module quiz Pull requests that affect the corresponding module text Pull requests that affect the corresponding module labels Oct 23, 2024
*/
@GetMapping("exam/exercises/{exerciseId}")
@EnforceAtLeastEditor
public ResponseEntity<Exercise> getExamExercise(@PathVariable Long exerciseId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency from exam->specific exercise should be fine for now, as one needs to know which exercises are created for an exam


// Exam exercise
Exercise exercise = exerciseRepository.findByIdElseThrow(exerciseId);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could reduce the repository call to just fetch the exercise type

return authCheckService.isAllowedToSeeExercise(exercise, user);
}

public boolean isCorrectExerciseType(Exercise exercise, Class<? extends Exercise> expectedExerciseClass) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not be needed here, we don't gain a lot from sharing this method.

return expectedExerciseClass.isInstance(exercise);
}

public void setChannelName(Exercise exercise) {
Copy link
Contributor Author

@ole-ve ole-ve Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to improve the naming here. For me, it's something similar to hydrateWithXY or loadAndSetXY to express that we are loading something from the DB.

Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exam Pull requests that affect the corresponding module exercise Pull requests that affect the corresponding module fileupload Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module quiz Pull requests that affect the corresponding module server Pull requests that update Java code. (Added Automatically!) stale text Pull requests that affect the corresponding module
Projects
Status: Work In Progress
Development

Successfully merging this pull request may close these issues.

1 participant